Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(argocd-image-updater): made argocd image updater cm and secret name templated #2998

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

KyriosGN0
Copy link

Signed-off-by: AvivGuiser [email protected]

Checklist:

  • I have bumped the chart version according to versioning
  • I have updated the documentation according to documentation
  • I have updated the chart changelog with all the changes that come with this pull request according to changelog.
  • Any new values are backwards compatible and/or have sensible default.
  • I have signed off all my commits as required by DCO.
  • My build is green (troubleshooting builds).

@mkilchhofer
Copy link
Member

Hi @KyriosGN0
May I ask for your motivation to template the ConfigMap/Secret?
Especially the two resources argocd-image-updater-secret and argocd-image-updater-config should be "hardcodeded" since we strictly follow the upstream projects.

And it is mentioned several times inside AUI's documentation

@KyriosGN0
Copy link
Author

KyriosGN0 commented Oct 30, 2024

we have a single argocd instance but we have 3 different image updaters, i don't want them to share the same secret/cm as they supposed to be managed in argo and if three different applications will try to control the same cm it won't work
@mkilchhofer

@KyriosGN0
Copy link
Author

@mkilchhofer i understand that you follow the upstream project, however i belive that in this case we preserve the purpose of those cm by adding the suffixes to the end of the templated name

@mkilchhofer
Copy link
Member

@mkilchhofer i understand that you follow the upstream project, however i belive that in this case we preserve the purpose of those cm by adding the suffixes to the end of the templated name

My only concern is that the name changes depending on the helm command our users are using (helm's release-name). E.g.:

  • helm install aui argo/argocd-image-updater -> with your implementation, the CMs are then named:
    • aui-argocd-image-updater
    • aui-argocd-image-updater-sshconfig
    • aui-argocd-image-updater-config
  • helm install image-updater argo/argocd-image-updater -> with your implementation, the CMs are then named:
    • image-updater-argocd-image-updater
    • image-updater-argocd-image-updater-sshconfig
    • image-updater-argocd-image-updater-config

Maybe using 3 top-level variables with a static name would fit better? Opinions, @yu-croco @jmeridth @tico24 @mbevc1 ?

@KyriosGN0
Copy link
Author

@mkilchhofer im probably missing something, what is the issue with those names being different ? the deployment will just pull the new names (with their data unchanged)

Signed-off-by: AvivGuiser <[email protected]>
@KyriosGN0
Copy link
Author

Hey @mkilchhofer is there any info I can give to expedite the decision ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants